-
Notifications
You must be signed in to change notification settings - Fork 9
Add new mode for SF average operation #2565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@timjarsky Here is a first version to play with: The new mode is called "sweepgroup". For this mode avg requires as first argument an array of at least two selections. What it does is the following:
Selection to sweep data:
averaging now is done along the rows:
The non existing sweeps in a row are ignored. The columns in the upper table are the sweep data that was retrieved for a given selection. |
|
@t-b @MichaelHuth does this need to be limited to select operations as input? For example, I'd like to pass in apfrequency data (see sample SF data below). |
|
@timjarsky I made a change such that any dataset type elements for the first argument are accepted. For your sketched case the last line must be changed to Now generally any input in the format If the dataset is of type selection then automatically It might also make sense to rename the new mode from The logic as sketched with the selections and the table above also applies for the generic dataset input for matching what gets averaged. |
|
Hi @MichaelHuth , I tested The end goal is to input multiple FI curves from individual experiments, and generate an average FI response (multiple average frequency vs average current data points) |
|
@timjarsky I made a change such that meta data like the sweep numbers is transferred from the first group in the array to the average result:
The distribution on the x-axis results from sweep number meta data present in the result wave. In your data the two incoming groups have different sweep numbers, so on a numerical basis it does not make sense to join the sweep numbers somehow together. The only other option would be to create some text wave where the sweep numbers of the incoming groups are joined like e.g. |
|
@MichaelHuth this question is tangental to this PR. I expect the following three code snippets to return the same thing, but one doesn't. Am I misunderstanding what the "intersection" of select operations means?
do |
The range does not intersect. This is also in the select documentation: The background here is that selrange can contain either epoch specifications with optional wildcards or time intervals. The actual resolve of the epoch to time intervals is done when data is applied (i.e. when the sweep data is retrieved). But let's say we would already do that part of data in the select statement and resolve the time intervals. Then the next issue comes up and this is the question how to exactly do the range intersection? If there is an inner selrange("E*") that would resolve to time intervals [100, 200] and [300, 400] and then there is an outer selrange to intersect with e.g. [150, 350] and [380, 500] then I can intersect every range with every other: Is this really useful for sweep data? Another approach would be to do intersection through epoch names by the names, if both sides of the intersection contain the same name. e.g. "E*" would possibly resolve to "E1", "E2", "E3". Intersected with "E2" results in "E2". But what about "E2" with "ST"? So in summary, intersecting ranges with selects currently not done. It is in principle possible, if there is a clear definition how this intersection should operate as it is not straight forward. It would require additional changes in SF for select because the actual ranges would need to be determined earlier (now: in data, then additionally: in each select). It would make select slower. |
|
Hi @MichaelHuth , thank you for the clear explanation. I define the intersection as the set of regions that overlap. So I would do the following: [100, 200] with [150, 350] -> [150, 200] However, I can work with it as it is and would want to talk more before implementing anything. Thanks again. |
|
Hi @MichaelHuth, the latest changes may have broken averaging; I expect the bottom plot to contain the average of the to FI arrays in the top plot.
|
|
@MichaelHuth okay, the latest change is helpful:
Now I want to subtract the first value in $currentA from all of $currentA. I imagined the syntax like this: $currentA - $currentA[0]. Is there a way to do that? |
|
At the moment there is not. We have an operation dataset to join data to a dataset, but we have no operation to extract a single dataset from an expression that returns multiple datasets. Indexing through brackets is not supported by SF. Adding indexing support with brackets to SF would be a big change because the parser/executor would need changes for that. A way with less effort is to introduce a new operation like Such that you can write I will add an operation for this, so you can progress. |
|
@timjarsky I added the extract operation that allows to extract a single dataset from a multi dataset expression. Your substraction expression looks like: I noticed that there is historically no explicit discrimination between arrays and datasets in SF. Thus, for some constructs the executor joins datasets to an array, especially when entered directly (not as result of an operation). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds a new "sweepgroup" mode for the SF average operation and introduces a new extract operation for extracting elements from datasets.
- Adds
SF_OP_AVG_SWEEPGROUPSmode to theavgoperation, enabling averaging across sweep groups - Implements the
extractoperation to retrieve specific indices from datasets - Refactors helper functions to support select-type argument resolution and dataset array handling
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| UTF_SweepFormula_Operations.ipf | Adds test coverage for the new extract operation |
| MIES_SweepFormula_PSX.ipf | Removes unused opShort parameter from SFH_GetArgumentSelect calls |
| MIES_SweepFormula_Operations_TP.ipf | Removes unused opShort parameter from SFH_GetArgumentSelect call |
| MIES_SweepFormula_Operations.ipf | Implements new sweepgroup mode for avg operation, adds extract operation, and removes redundant scale operations |
| MIES_SweepFormula_Helpers.ipf | Extends helper functions to support select resolution and dataset array handling |
| MIES_SweepFormula_Executor.ipf | Adds case handler for new extract operation |
| MIES_SweepFormula.ipf | Registers the new extract operation |
| MIES_Constants.ipf | Defines constant for the new extract operation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The background for this is the following: The Now we have some logic where we look at the meta data of a result in the plotter to determine how to display it on an x-axis. One thing evaluated is, if the data is a single data point and the meta data contains a sweep number. Then the plot with the known colored dots is created (as seen above). Now in the data from the example experiment data from sweep 19 is averaged with data from sweep 69, 20 with 70 and so on.
And this sketches only possibilities if all groups have sweep numbers, what if one group has sweep numbers and the next has some different meta data property? Currently I implemented point 1. |
Yes I think this is fixed in main. |
- opShort was not used
In default setting the function enforces that at the given position is a select type expression or array of it. It either gives an assertion or defaults to the standard select expression select(). When doNotEnforce is set then the function returns a null wave if any condition is not met that the content of the argument is actually a select type. This allows for a caller to attempt a resolution of the argument as select type data (or array of it) and handle it differently if the content is of a different type.
Just refactor, no functional changes.
…ion to data If the first argument of avg is a selection or an array of selections it is now automatically converted to the sweep data selected.
c4d3f89 to
f0e5dc8
Compare
- a new mode was added "group" - the user has to give an array of datasets with at least 2 elements as first argument - the averaging is done over the first entries of the datasets, then over the second and so on ds0 = dataset(1, 2, 3) ds1 = dataset(4, 5, 6) avg([$ds1, $ds2], group) Then 1 is averaged with 4 2 is averaged with 5 3 is averaged with 6 and the resulting dataset contains three elements with single point waves equal to dsResult = dataset(2.5, 3.5, 4.5)
…s result The API is: extract(multi_dataset, index) extract returns a single dataset. Added test
to rst and SF help notebook
f0e5dc8 to
32a7878
Compare
|
I renamed the mode to just |







close #2496